-
Notifications
You must be signed in to change notification settings - Fork 77
Model class hierarchy change for select nodes #3670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Please fix all failures. Your change breaks the Yosys plugin code it looks like. |
0faf554 to
9b3c7b8
Compare
|
@tgorochowik @hzeller Could either of you assist with the necessary changes to the yosys plugin codebase |
|
Can you open a feature request on the yosys plugin and describe what changes in the UHDM data structure would happen after this is submitted ? Then the relevant implementation details can be discussed there. |
|
Also massive diffs need to be updated here |
|
UHDM model hierarchy changed to enforce vpiParent as weak reference.
|
@alaindargelas @hzeller This is ready for review and merge. I have open PR chipsalliance/yosys-f4pga-plugins#522 to fix the plugin issue. The non-vendor build will continue to fail until UHDM PR is merged and tag updated. Also, as a proof of concept I have #3681 with this change and the change in plugin repo as a patch. The plugin build succeeded. |
f22cf2e to
b8ed6d0
Compare
|
Input: module top(output int o);
typedef struct packed {
logic [9:0] min_v;
} filter_ctl_t;
filter_ctl_t [1:0][2:0] a;
assign a[0][0].min_v = '1;
endmoduleUHDM diff between master and the PR: --- surelog-a85ff7c1dcf411ff1b7b2b48b5dad17224885197/uhdm.txt 2023-06-02 13:39:12.983968769 +0200
+++ surelog-b8ed6d0a4485f39762c96e60036036a34282efdf/uhdm.txt 2023-06-02 13:39:12.991968881 +0200
@@ -465,24 +465,24 @@
|vpiParent:
\_cont_assign: , line:7:11, endln:7:29
|vpiName:a[0][0].min_v
|vpiActual:
- \_bit_select: (a), line:7:11, endln:7:12
+ \_bit_select: ([email protected][0][0].min_v.a), line:7:11, endln:7:12
|vpiParent:
- \_ref_obj: ([email protected][0])
- |vpiParent:
- \_hier_path: (a[0][0].min_v), line:7:11, endln:7:12
- |vpiName:a[0]
- |vpiFullName:[email protected][0]
+ \_hier_path: (a[0][0].min_v), line:7:11, endln:7:12
|vpiName:a
+ |vpiFullName:[email protected][0][0].min_v.a
|vpiIndex:
\_constant: , line:7:13, endln:7:14
|vpiDecompile:0
|vpiSize:64
|UINT:0
|vpiConstType:9
|vpiActual:
- \_bit_select: , line:7:13, endln:7:14
+ \_bit_select: ([email protected][0][0].min_v), line:7:13, endln:7:14
+ |vpiParent:
+ \_hier_path: (a[0][0].min_v), line:7:11, endln:7:12
+ |vpiFullName:[email protected][0][0].min_v
|vpiIndex:
\_constant: , line:7:16, endln:7:17
|vpiDecompile:0
|vpiSize:64
@@ -586,19 +586,17 @@
|vpiParent:
\_cont_assign: , line:7:11, endln:7:29
|vpiName:a[0][0].min_v
|vpiActual:
- \_bit_select: (a), line:7:11, endln:7:12
+ \_bit_select: (a[0][0].min_v.a), line:7:11, endln:7:12
|vpiParent:
- \_ref_obj: (a[0])
- |vpiParent:
- \_hier_path: (a[0][0].min_v), line:7:11, endln:7:12
- |vpiName:a[0]
+ \_hier_path: (a[0][0].min_v), line:7:11, endln:7:12
|vpiName:a
+ |vpiFullName:a[0][0].min_v.a
|vpiIndex:
\_constant: , line:7:13, endln:7:14
|vpiParent:
- \_bit_select: (a), line:7:11, endln:7:12
+ \_bit_select: (a[0][0].min_v.a), line:7:11, endln:7:12
|vpiDecompile:0
|vpiSize:64
|UINT:0
|vpiConstType:9I guess On a side note, both trees seem to be invalid, see: #3691 |
UHDM model hierarchy changed to enforce vpiParent as weak reference.
bit_select, and other select classes use the vpiParent pointer (as refobj) to provide parent and actual_group access. bit_select.vpiParent.vpiParent is the "true" parent of the bit_select and bit_select.vpiParent.actual_group is access to the actual. However, vpiParent edges in the graph are weak references and ignored for all traversals (including VpiListener, UhdmListener, and vpi_visitor). Tail of parent edges are missing in the UHDM output since the nodes are ignored. The traversal mode also generate unexpected results during runtime because of the ignored edges. Changing the hierarchy so that ref_obj works as a base (intermediate class in the class hierarchy, yet instantiable) and other select classes (including bit_select, indexed_part_select, part_select, and var_select) are subclasses of ref_obj. This gives access to ref_obj.actual_group and ref.vpiParent can be used for the usual parenting purposes.
Model class hierarchy change for select nodes
bit_select, and other select classes use the vpiParent pointer (as refobj) to provide parent and actual_group access. bit_select.vpiParent.vpiParent is the "true" parent of the bit_select and bit_select.vpiParent.actual_group is access to the actual. However, vpiParent edges in the graph are weak references and ignored for all traversals (including VpiListener, UhdmListener, and vpi_visitor). Tail of parent edges are missing in the UHDM output since the nodes are ignored. The traversal mode also generate unexpected results during runtime because of the ignored edges.
Changing the hierarchy so that ref_obj works as a base (intermediate class in the class hierarchy, yet instantiable) and other select classes (including bit_select, indexed_part_select, part_select, and var_select) are subclasses of ref_obj. This gives access to ref_obj.actual_group and ref.vpiParent can be used for the usual parenting purposes.
Updated class file generation logic to support generating class files for intermediate nodes in the class hierarchy.
Updated vpi_visitor, ExprEval, and clone_tree logic to compensate for the model hierarchy change.
Few code improvements in ElaboratorListener and BaseClass
Moved UhdmId, VpiFile, and VpiParent to BaseClass since these are required properties for all models in the class hierarchy